Closed
Bug 1258305
Opened 8 years ago
Closed 8 years ago
Pull ViewHelpers.L10N, MultiL10N into their own module
Categories
(DevTools :: Performance Tools (Profiler/Timeline), defect, P3)
DevTools
Performance Tools (Profiler/Timeline)
Tracking
(firefox48 fixed)
RESOLVED
FIXED
Firefox 48
Tracking | Status | |
---|---|---|
firefox48 | --- | fixed |
People
(Reporter: vporof, Assigned: vporof)
Details
(Keywords: addon-compat)
Attachments
(2 files, 1 obsolete file)
9.16 KB,
patch
|
jsantell
:
review+
|
Details | Diff | Splinter Review |
55.77 KB,
patch
|
jsantell
:
review+
|
Details | Diff | Splinter Review |
ViewHelpers is a monster. Need to slim it down.
Assignee | ||
Updated•8 years ago
|
Summary: Pull ViewHelpers.L10N, MultiL10N and Prefs into their own modules → Pull ViewHelpers.L10N, MultiL10N into their own module
Assignee | ||
Comment 1•8 years ago
|
||
Assignee | ||
Comment 2•8 years ago
|
||
Files worthy to look at are ViewHelpers.jsm and devtools/client/shared/l10n.js Everything else is busywork.
Assignee | ||
Comment 3•8 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=cfdbce1cb8de
Assignee | ||
Comment 4•8 years ago
|
||
Attachment #8732790 -
Attachment is obsolete: true
Attachment #8732790 -
Flags: review?(jsantell)
Attachment #8732822 -
Flags: review?(jsantell)
Assignee | ||
Comment 5•8 years ago
|
||
Attachment #8732823 -
Flags: review?(jsantell)
Assignee | ||
Comment 6•8 years ago
|
||
(In reply to Victor Porof [:vporof][:vp] from comment #2) > Files worthy to look at are ViewHelpers.jsm and > devtools/client/shared/l10n.js > Everything else is busywork. Split that giant patch into two smaller ones for easier review.
Assignee | ||
Comment 7•8 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=d46ec09bcb8f
Assignee | ||
Comment 8•8 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=5cbf9fdf9d44
Assignee | ||
Comment 9•8 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=67c6e7083883
Updated•8 years ago
|
Attachment #8732822 -
Flags: review?(jsantell) → review+
Comment 10•8 years ago
|
||
Comment on attachment 8732823 [details] [diff] [review] v1 busywork Review of attachment 8732823 [details] [diff] [review]: ----------------------------------------------------------------- ::: devtools/client/inspector/layout/layout.js @@ +9,5 @@ > const {Cc, Ci, Cu} = require("chrome"); > const {InplaceEditor, editableItem} = > require("devtools/client/shared/inplace-editor"); > const {ReflowFront} = require("devtools/server/actors/layout"); > +const {Localization} = require("devtools/client/shared/l10n"); nit spacing ::: devtools/client/netmonitor/netmonitor-view.js @@ +21,5 @@ > > const {ToolSidebar} = require("devtools/client/framework/sidebar"); > const {Tooltip} = require("devtools/client/shared/widgets/Tooltip"); > const DevToolsUtils = require("devtools/shared/DevToolsUtils"); > +const {Localization} = require("devtools/client/shared/l10n"); nit spacing ::: devtools/client/responsive.html/components/utils/l10n.js @@ +1,1 @@ > /* This Source Code Form is subject to the terms of the Mozilla Public This file might be redundant now ::: devtools/client/shared/source-utils.js @@ +5,5 @@ > > const { URL } = require("sdk/url"); > +const { Localization } = require("devtools/client/shared/l10n"); > + > +const l10n = new Localization("chrome://devtools/locale/components.properties"); Lower case `l10n` was used because the previous import was `L10N` -- maybe this should always be uppercased now for consistency across tools ::: devtools/client/shared/test/browser_flame-graph-04.js @@ +3,5 @@ > > // Tests that text metrics in the flame graph widget work properly. > > var HTML_NS = "http://www.w3.org/1999/xhtml"; > +var {Localization} = require("devtools/client/shared/l10n"); nit `{ Localization }` ::: devtools/client/shared/widgets/VariablesViewController.jsm @@ +12,5 @@ > Cu.import("resource://devtools/client/shared/widgets/ViewHelpers.jsm"); > var {require} = Cu.import("resource://devtools/shared/Loader.jsm", {}); > var Services = require("Services"); > var promise = require("promise"); > +var {Localization} = require("devtools/client/shared/l10n"); nit spacing ::: devtools/client/storage/ui.js @@ +6,5 @@ > "use strict"; > > const {Cu} = require("chrome"); > const EventEmitter = require("devtools/shared/event-emitter"); > +const {Localization} = require("devtools/client/shared/l10n"); nit spacing
Attachment #8732823 -
Flags: review?(jsantell) → review+
Assignee | ||
Comment 11•8 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/cc08584692775437121919c69adcd4fc0cfb97f7 Bug 1258305 - Pull ViewHelpers.L10N, MultiL10N into their own module, r=jsantell https://hg.mozilla.org/integration/fx-team/rev/b8b21810b2b3b42a90dfda07c3ff0ecf1a030727 Bug 1258305 - Update all occurences of ViewHelpers.L10N and MultiL10N to use the new module, r=jsantell
Assignee | ||
Updated•8 years ago
|
Priority: -- → P3
Comment 12•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/cc0858469277 https://hg.mozilla.org/mozilla-central/rev/b8b21810b2b3
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox48:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 48
Comment 13•8 years ago
|
||
Marking addon-compat because it broke my 2 devtools addons.
Keywords: addon-compat
Updated•6 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•